Don't encode precise in lockfile dep pointers

Each dependency itself already has the precise source listed, and we're
guaranteed that for each (source, name) pair that there is exactly one
dependency, so there is always a way to rebuild the precise dependency graph
after the fact by looking up the (source, name) pair in the hash map.

This should help with some of the readability concerns in #637 because the git
SHA that a source is locked to is now only mentioned once in a lockfile.

This commit does preserve, however, the mention of the version in each
dependency line which will likely never go away. This means that for
registry-based packages will still run into the same lockfile merge conflict
troubles, there will just be more readable versions than git hashes.

It should also be noted that this will alter all currently generated lockfiles
as any dependencies mentioned will lose the hashes mentioned afterwards. This
will likely cause somewhat of a transitionary pain as this version of cargo
propagates throughout.

diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs
index f9ee93b4030d9b8ae6544aa891147a83ce7a70f3..fffb9be84e748ddf33d8e3a595ec5454bd16003d 100644 (file)
--- a/src/cargo/core/package_id.rs
+++ b/src/cargo/core/package_id.rs
@@ -132,6 +132,16 @@ impl PackageId {
 
         Metadata { metadata: metadata, extra_filename: extra_filename }
     }
+
+    pub fn with_precise(&self, precise: Option<String>) -> PackageId {
+        PackageId {
+            inner: Arc::new(PackageIdInner {
+                name: self.inner.name.to_string(),
+                version: self.inner.version.clone(),
+                source_id: self.inner.source_id.with_precise(precise),
+            }),
+        }
+    }
 }
 
 impl Metadata {

diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs
index ec684d8725015cc22aa7cc28719faf9f3553cd72..152be313ddafe18b5c236b8ce952d9d506e3f070 100644 (file)
--- a/src/cargo/core/resolver/encode.rs
+++ b/src/cargo/core/resolver/encode.rs
@@ -20,48 +20,62 @@ pub type Metadata = TreeMap<String, String>;
 impl EncodableResolve {
     pub fn to_resolve(&self, default: &SourceId) -> CargoResult<Resolve> {
         let mut g = Graph::new();
+        let mut tmp = HashMap::new();
+
+        let packages = Vec::new();
+        let packages = self.package.as_ref().unwrap_or(&packages);
+
+        {
+            let register_pkg = |pkg: &EncodableDependency| {
+                let pkgid = try!(pkg.to_package_id(default));
+                let precise = pkgid.get_source_id().get_precise()
+                                   .map(|s| s.to_string());
+                assert!(tmp.insert(pkgid.clone(), precise),
+                        "a package was referenced twice in the lockfile");
+                g.add(try!(pkg.to_package_id(default)), []);
+                Ok(())
+            };
+
+            try!(register_pkg(&self.root));
+            for pkg in packages.iter() {
+                try!(register_pkg(pkg));
+            }
+        }
 
-        try!(add_pkg_to_graph(&mut g, &self.root, default));
-
-        match self.package {
-            Some(ref packages) => {
-                for dep in packages.iter() {
-                    try!(add_pkg_to_graph(&mut g, dep, default));
+        {
+            let add_dependencies = |pkg: &EncodableDependency| {
+                let package_id = try!(pkg.to_package_id(default));
+
+                let deps = match pkg.dependencies {
+                    Some(ref deps) => deps,
+                    None => return Ok(()),
+                };
+                for edge in deps.iter() {
+                    let to_depend_on = try!(edge.to_package_id(default));
+                    let precise_pkgid =
+                        tmp.find(&to_depend_on)
+                           .map(|p| to_depend_on.with_precise(p.clone()))
+                           .unwrap_or(to_depend_on.clone());
+                    g.link(package_id.clone(), precise_pkgid);
                 }
+                Ok(())
+            };
+
+            try!(add_dependencies(&self.root));
+            for pkg in packages.iter() {
+                try!(add_dependencies(pkg));
             }
-            None => {}
         }
 
-        let root = self.root.to_package_id(default);
         Ok(Resolve {
             graph: g,
-            root: try!(root),
+            root: try!(self.root.to_package_id(default)),
             features: HashMap::new(),
             metadata: self.metadata.clone(),
         })
     }
 }
 
-fn add_pkg_to_graph(g: &mut Graph<PackageId>,
-                    dep: &EncodableDependency,
-                    default: &SourceId)
-                    -> CargoResult<()>
-{
-    let package_id = try!(dep.to_package_id(default));
-    g.add(package_id.clone(), []);
-
-    match dep.dependencies {
-        Some(ref deps) => {
-            for edge in deps.iter() {
-                g.link(package_id.clone(), try!(edge.to_package_id(default)));
-            }
-        },
-        _ => ()
-    };
-
-    Ok(())
-}
-
 #[deriving(Encodable, Decodable, Show, PartialOrd, Ord, PartialEq, Eq)]
 pub struct EncodableDependency {
     name: String,
@@ -178,7 +192,7 @@ fn encodable_package_id(id: &PackageId, root: &PackageId) -> EncodablePackageId
     let source = if id.get_source_id() == root.get_source_id() {
         None
     } else {
-        Some(id.get_source_id().clone())
+        Some(id.get_source_id().with_precise(None))
     };
     EncodablePackageId {
         name: id.get_name().to_string(),